Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[17509] Feature: Ignore empty API #3354

Merged
merged 19 commits into from
Mar 21, 2023
Merged

[17509] Feature: Ignore empty API #3354

merged 19 commits into from
Mar 21, 2023

Conversation

JLBuenoLopez
Copy link
Contributor

@JLBuenoLopez JLBuenoLopez commented Mar 10, 2023 •
edited
Loading

Description

This PR introduces the required API to implement the ignore DDS entity feature.

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • [N/A] Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • Any new/modified methods have been properly documented using Doxygen.
  • [NO] Changes are ABI compatible.
  • [NO] Changes are API compatible.
  • New feature has been added to the versions.md file (if applicable).
  • [N/A] New feature has been documented/Current behavior is correctly described in the documentation.
  • [N/A] Applicable backports have been included in the description.

As this feature is not yet implemented, it will be documented in a follow-up PR when the implementation is made.

Reviewer Checklist

  • The PR has a milestone assigned.
  • Check contributor checklist is correct.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

@JLBuenoLopez JLBuenoLopez added no-test Skip CI tests if PR marked with this label doc-pending Issue or PR which is pending to be documented labels Mar 10, 2023
@JLBuenoLopez
Copy link
Contributor Author

Uncrustify fails because it tries to remove a required ; from an enum declaration.

@JLBuenoLopez
Copy link
Contributor Author

@richiprosima please test this

@JLBuenoLopez JLBuenoLopez added this to the v2.10.0 milestone Mar 10, 2023
@JLBuenoLopez
Copy link
Contributor Author

@richiprosima please test mac

@JLBuenoLopez
Copy link
Contributor Author

@richiprosima please test this

@JLBuenoLopez JLBuenoLopez removed the no-test Skip CI tests if PR marked with this label label Mar 10, 2023
@JLBuenoLopez
Copy link
Contributor Author

JLBuenoLopez commented Mar 10, 2023 •
edited
Loading

Running the test suite because one mock has been modified in order to fix a Mac warning.

@JLBuenoLopez
Copy link
Contributor Author

@richiprosima please test this

@JLBuenoLopez
Copy link
Contributor Author

Test failure does not seem related.

MRicoIE2CS
MRicoIE2CS suggested changes Mar 16, 2023
View reviewed changes
include/fastdds/dds/domain/DomainParticipantListener.hpp Show resolved Hide resolved
include/fastdds/dds/domain/DomainParticipantListener.hpp Show resolved Hide resolved
src/cpp/fastdds/domain/DomainParticipantImpl.hpp Show resolved Hide resolved
src/cpp/fastdds/domain/DomainParticipantImpl.hpp Show resolved Hide resolved
src/cpp/fastdds/domain/DomainParticipantImpl.hpp Show resolved Hide resolved
include/fastdds/dds/domain/DomainParticipantListener.hpp Show resolved Hide resolved
include/fastdds/rtps/builtin/discovery/participant/PDP.h Outdated Show resolved Hide resolved
include/fastdds/rtps/participant/RTPSParticipant.h Outdated Show resolved Hide resolved
include/fastdds/rtps/participant/RTPSParticipantListener.h Outdated Show resolved Hide resolved
src/cpp/rtps/participant/RTPSParticipantImpl.h Outdated Show resolved Hide resolved
MRicoIE2CS
MRicoIE2CS suggested changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@MRicoIE2CS MRicoIE2CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are Uncrustify issues.

MRicoIE2CS
MRicoIE2CS suggested changes Mar 16, 2023
View reviewed changes
Copy link
Contributor

@MRicoIE2CS MRicoIE2CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The branch has conflicts in the versions file. A rebase would be needed before merge.

@JLBuenoLopez
Copy link
Contributor Author

There are Uncrustify issues.

Please, read my comment above about this issue.

@JLBuenoLopez
Refs #17509: RTPSParticipantImpl empty ignore API
79ba117 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: RTPSParticipant public empty ignore API
59d93c9 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: return boolean to propagate error to upper layer. Removi…
aa19cd2 
…ng the ProxyData may fail

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: remove warning
d64bed6 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: return boolean to propagate error to upper layer
694e02b 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: DomainParticipantImpl ignore API
fe8b09f 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: new discovery info reason: IGNORED
5580c5f 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: remove warning
9edd7fc 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: overload PDP:removeWriterProxyData and PDP:removeReaderP…
9ae2274 
…roxyData

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: linter
faecdd4 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: enforce code style
c9281c4 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: overload RTPS discovery callbacks
89b668d 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: enforce code style
52fb53e 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: overload DDS discovery callbacks
2a19d61 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: update versions.md
e573bf0 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: linters
87356fb 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: fix Mac warning
351a8ed 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez
Refs #17509: apply review suggestion: imporve Doxydoc
3a65317 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
MRicoIE2CS

This comment was marked as outdated.

Sign in to view

@MRicoIE2CS MRicoIE2CS added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. ci-pending PR which CI is running labels Mar 16, 2023
@JLBuenoLopez
Refs #17509: fix documentation CI
206f2ca 
Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez JLBuenoLopez removed doc-pending Issue or PR which is pending to be documented ci-pending PR which CI is running labels Mar 17, 2023
MRicoIE2CS
MRicoIE2CS approved these changes Mar 17, 2023
View reviewed changes
Copy link
Contributor

@MRicoIE2CS MRicoIE2CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MiguelCompany
Copy link
Member

@richiprosima Please test this

@EduPonz EduPonz merged commit 46c891e into master Mar 21, 2023
@EduPonz EduPonz deleted the feature/ignore-empty-api branch March 21, 2023 11:44
@JLBuenoLopez JLBuenoLopez mentioned this pull request Mar 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MRicoIE2CS MRicoIE2CS MRicoIE2CS approved these changes

Assignees
No one assigned
Labels
ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Milestone
v2.10.0
Development

Successfully merging this pull request may close these issues.
4 participants
@JLBuenoLopez @MiguelCompany @MRicoIE2CS @EduPonz